feat: nested ptys, forward sigwinch, keychain access by caarlos0 · Pull Request #322 · microsoft/mxc

caarlos0 · 2026-05-15T15:57:40Z

📖 Description

Three independent fixes that together unblock interactive agent workflows under the macOS seatbelt backend. The baseline profile breaks anything that needs to spawn a real shell (tests, git, gh, REPLs) or talk to the system Keychain, and the pty bridge corrupts TUIs whenever the host wraps mxc-exec-mac in its own pty.

feat(seatbelt): add nestedPty + keychainAccess experimental options

Two new opt-in/opt-out fields under experimental.seatbelt:

nestedPty (default true): allow the inner workload to call posix_openpt. Emits (allow pseudo-tty), (allow iokit-open), and read/write/ioctl on /dev/ptmx. Default-on because almost any agent shell tool needs it.
keychainAccess (default false): allow Security.framework/keytar to reach the system keychain stack. Emits Mach lookups for SecurityServer, securityd, trustd, cfprefsd.daemon, xpcd, and the com.apple.lsd.* family (via global-name-regex); read access to /Library/Keychains + /private/var/db/mds; read+write on $HOME/Library/Keychains + /private/var/folders; and (allow iokit-open).

Both blocks are emitted before write_ui_rules so the HID iokit-open deny still wins under Seatbelt's last-match-wins semantics. nestedPty is suppressed when guiAccess already emits a strict superset.

fix(pty): make nested ptys work end-to-end under macOS

Four gaps in mxc_pty::run_with_pty that broke any nested-pty workload under a host-allocated outer pty:

Outer pty slave was left in cooked mode → input bytes echoed back, control chars rendered as ^X. Added a RAII RawSlaveGuard that puts the outer slave into raw mode and restores termios on drop. No-op when fd 0 isn't a tty.
Inner pty was created at the kernel default 0×0. Now inherits the outer pty's TIOCGWINSZ.
SIGWINCH was never propagated. Added a self-pipe SIGWINCH handler + a forwarder thread that re-reads the outer winsize and pushes it to the inner pty master via TIOCSWINSZ. Self-pipe rather than sigwait so we don't depend on which thread the runtime lets the signal land on.
Inner child inherited the parent's sigmask (signal_cleanup blocks signals via sigwait). pre_exec now unconditionally adds SIGWINCH to the unblock set — execve resets the handler but preserves the inherited mask.

🔗 References

https://github.com/github/copilot-agent-runtime/issues/164

🔍 Validation

cargo fmt --all -- --check clean.
cargo clippy -p mxc_pty -p seatbelt_common -p wxc_common -p mxc_darwin --all-targets -- -D warnings clean.
cargo test -p mxc_pty -p seatbelt_common -p wxc_common — 4 + 33 + 190 = 227 tests pass. New unit tests cover: nestedPty defaults / on / off / interaction with guiAccess / HID deny ordering; keychainAccess mach + FS + iokit + HID deny ordering; parser pass-through of both fields.
Manual: ran Copilot CLI inside the seatbelt sandbox on macOS — gh, git, node REPLs, and resize-on-the-fly all work; keytar reads/writes Keychain entries with keychainAccess: true.

✅ Checklist

Signed the Contributor License Agreement
Linked to an issue
Updated documentation (if applicable)
Updated Copilot instructions (if build, architecture, or conventions changed)

📋 Issue Type

Bug fix
Feature
Task

macOS' default 256-soft NOFILE cap is too low for Node and other agent runtimes that open many file descriptors (sockets, ptys, file watchers) inside the seatbelt sandbox; they hit EMFILE under load. Raise the soft limit to the kernel hard cap on startup (best-effort) so the inner workload gets the full available headroom. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

Two new opt-in/opt-out fields under `experimental.seatbelt` to unblock real agent workflows that the baseline profile breaks: - `nestedPty` (default `true`): allow the inner workload to call `posix_openpt` and friends. Emits `pseudo-tty`, broad `(allow iokit-open)`, and read/write/ioctl on `/dev/ptmx`. Without this anything that spawns a real shell (tests, git, gh, REPLs) dies inside the sandbox because `TTY_ALLOW` only covers the outer pty. - `keychainAccess` (default `false`): allow Security.framework / keytar to talk to the system keychain stack. Emits Mach lookups for SecurityServer, securityd, cfprefsd.daemon, xpcd, and the `com.apple.lsd.*` family (via `global-name-regex`); read access to `/Library/Keychains` and `/private/var/db/mds`; read+write on `$HOME/Library/Keychains` and `/private/var/folders`; and `(allow iokit-open)` (redundant when `nestedPty` is on, but keeps the capability self-contained). Both rule blocks are emitted before `write_ui_rules` so the HID `iokit-open` deny still wins under Seatbelt's last-match-wins semantics, and `nestedPty` is suppressed when `guiAccess` already emits a strict superset. Includes schema, docs, and unit tests covering defaults, on/off, interaction with `guiAccess` and `ui.disable`, and HID deny ordering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

The pty bridge had several gaps that broke any nested-pty workload (inner shell + its own children — e.g. node TUIs, REPLs, anything that tcgetattr-s its tty) when run under the seatbelt sandbox via a host that wraps mxc-exec-mac in its own outer pty: - The outer pty slave (our fd 0) was left in cooked mode, so input bytes were echoed back and control chars rendered as `^X` on the way to the inner child, corrupting any TUI it tried to render. Install a RAII RawSlaveGuard that puts the outer slave into raw mode for the duration of the bridge and restores the original termios on drop. No-op when fd 0 is not a tty. - The inner pty was created with the kernel default 0×0 winsize, so TUIs sized to a zero-by-zero terminal. Inherit the outer pty's TIOCGWINSZ when available. - SIGWINCH was never propagated. The kernel delivers it to us (because fd 0 is the outer slave), but we did nothing with it — the inner child never resized. Install a self-pipe SIGWINCH handler + a forwarder thread that re-reads the outer winsize and pushes it to the inner pty master via TIOCSWINSZ on every event. Self-pipe rather than sigwait so we don't depend on which thread the runtime lets the signal land on. - The inner child inherited whatever sigmask the parent had installed (signal_cleanup blocks a bunch via sigwait). Add SIGWINCH to the unblock set unconditionally in pre_exec — execve resets the handler to default but preserves the inherited mask, so a child that installs its own SIGWINCH handler (node, neovim, …) would otherwise silently never see it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

This reverts commit 6b8c32d.

Copilot

Pull request overview

This PR extends the macOS Seatbelt backend with nested PTY and optional Keychain access support, and improves the shared Unix PTY bridge to propagate terminal sizing and raw-mode behavior for nested interactive sessions.

Changes:

Adds nestedPty and keychainAccess config fields for Seatbelt in Rust models, parser tests, schema, and macOS docs.
Emits additional Seatbelt profile rules for nested PTY allocation and Keychain-related Mach/filesystem access.
Updates mxc_pty to inherit window size, forward SIGWINCH, and set the outer PTY slave to raw mode.

Show a summary per file

File	Description
`src/wxc_common/src/models.rs`	Adds Seatbelt config fields and manual defaults.
`src/wxc_common/src/config_parser.rs`	Parses new Seatbelt fields and adds parser tests.
`src/seatbelt_common/src/profile_builder.rs`	Adds profile rules and tests for nested PTYs and Keychain access.
`src/mxc_pty/src/lib.rs`	Adds raw-mode handling, initial window sizing, and SIGWINCH forwarding.
`schemas/dev/mxc-config.schema.0.6.0-dev.json`	Documents new Seatbelt schema fields.
`docs/macos-support/seatbelt-backend.md`	Documents new Seatbelt options.

Copilot's findings

Comments suppressed due to low confidence (2)

src/mxc_pty/src/lib.rs:342

The SIGWINCH self-pipe is created in blocking mode, but the signal handler writes to it directly. If resize events arrive while the pipe is full or the reader is stalled, write(2) can block inside the signal handler and hang the process. Please make the write end non-blocking (and handle EAGAIN) before installing the handler.

    let (read_end, write_end) = nix::unistd::pipe().ok()?;
    let read_fd = read_end.as_raw_fd();
    let write_fd = write_end.as_raw_fd();
    // Leak the OwnedFds — they live for the rest of the process.
    // Closing them would race the signal handler.
    std::mem::forget(read_end);
    std::mem::forget(write_end);
    SIGWINCH_PIPE_WRITE_FD.store(write_fd, std::sync::atomic::Ordering::Release);

src/mxc_pty/src/lib.rs:352

Each run_with_pty call installs a new global SIGWINCH handler and overwrites the single static pipe fd. Previous resize threads and pipe fds are never shut down, and only the most recent invocation will receive future resize notifications. This makes repeated or concurrent pty runs leak resources and breaks resize forwarding for any earlier active session; the handler/pipe state needs to be installed once with per-session registration or cleaned up when the pty run completes.

    SIGWINCH_PIPE_WRITE_FD.store(write_fd, std::sync::atomic::Ordering::Release);

    // SIGWINCH's default action is "ignore", so without an installed
    // handler the kernel drops the signal entirely. SA_RESTART so we
    // don't break unrelated syscalls.
    unsafe {
        let mut sa: libc::sigaction = std::mem::zeroed();
        sa.sa_sigaction = sigwinch_handler as *const () as usize;
        libc::sigemptyset(&mut sa.sa_mask);
        sa.sa_flags = libc::SA_RESTART;
        if libc::sigaction(libc::SIGWINCH, &sa, std::ptr::null_mut()) != 0 {

Files reviewed: 6/6 changed files
Comments generated: 5

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

The Windows x64 CI lane runs `cargo test` and the keychain tests that set `keychain_access: true` exercise `write_keychain_rules`, which expands `~/Library/Keychains` from $HOME. Windows CI doesn't set $HOME so `build_profile().unwrap()` panics there. Keychain access only applies on macOS — gate the four tests that enable it to `cfg(target_os = "macos")`. The off-by-default test stays cross-platform because it never triggers the HOME lookup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

The SIGWINCH handler write()s to the self-pipe to wake the forwarder thread. If the pipe fills (reader stalled or pathological resize burst), a blocking write(2) inside an async-signal-safe handler can deadlock the process — and the existing comment already assumes EAGAIN-drop behavior that didn't actually hold without O_NONBLOCK. Set O_NONBLOCK on the write end before installing the handler. Best-effort: if fcntl fails we stay in blocking mode, same as before. Addresses Copilot reviewer feedback on #322. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

caarlos0 · 2026-05-15T16:51:10Z

Addressed Copilot reviewer feedback:

SIGWINCH self-pipe blocking write (mxc_pty/src/lib.rs:342): fixed in bf9baa0 — set O_NONBLOCK on the write end before installing the handler, so the handler can't deadlock if the pipe fills. The existing "drop on full pipe" comment now matches actual behavior.
Per-invocation global SIGWINCH state (mxc_pty/src/lib.rs:352): not addressing in this PR. The function lives in a single-shot binary (mxc-exec-mac and lxc-exec both call run_with_pty exactly once before exiting); there is no concrete caller doing repeated or concurrent runs. Fixing it correctly means either a once-init module-level handler or per-session registration, both of which add real complexity for no current user. Happy to revisit when there's a concrete caller that needs it.

Also fixed the x64 WXC CI failure in 6eb69e7 — the keychain tests need $HOME which isn't set on Windows CI; gated them to cfg(target_os = "macos") since the feature is macOS-only anyway.

…ychain

profile_builder is compiled (and unit-tested) on every host so reviewers can validate generated profiles without a Mac. `write_keychain_rules` expands `~/Library/Keychains` from $HOME, which Windows CI doesn't set — so on non-macOS the keychainAccess branch was either panicking in tests or silently emitting an invalid path. Have the builder simply ignore the option on non-macOS targets. Seatbelt itself only runs on macOS, so emitting nothing there is the correct behaviour, not a workaround. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

richiemsft · 2026-05-15T20:30:08Z

+    // posix_openpt → grantpt → ioctls bottom out in IOKit user-clients
+    // (IOSerialBSDClient and friends). Without iokit-open the kernel
+    // returns EPERM and the master fd is unusable.
+    out.push_str("(allow iokit-open)\n");


isn't this too broad? Is it possible to narrow it down to only what's needed?

Yes — verified empirically and dropped entirely in cb21f36. I tested on macOS 15 (Apple Silicon) under sandbox-exec with (allow pseudo-tty) + /dev/ptmx but no iokit-open: posix_openpt → grantpt → unlockpt → ptsname → slave open all succeed. Then end-to-end through mxc-exec-mac itself with the new profile builder — posix_openpt returns OK slave=/dev/ttys014, opened slave fd=7. So the rule is removed from write_nested_pty_rules along with the now-stale ordering test and build_profile comment.

richiemsft · 2026-05-15T20:31:18Z

+    out.push_str("    (subpath \"/private/var/folders\"))\n");
+
+    out.push_str(";; --- keychainAccess: iokit-open for crypto accelerators ---\n");
+    out.push_str("(allow iokit-open)\n");


same comment about iokit-open in here.

Same fix in cb21f36. Verified SecItemAdd + SecItemCopyMatching on a generic-password item under sandbox-exec with the keychain Mach surface (securityd, trustd, ocspd, cfprefsd, xpcd, lsd.*) and the FS surface (~/Library/Keychains, /private/var/folders, /private/var/db/mds) but no iokit-open — both return status 0. Dropped the rule. Also dropped the redundant /Library/Keychains and /System/Library/Keychains reads since the baseline (subpath "/Library") and (subpath "/System") already cover them, and updated the docs to match.

The forwarder was holding a borrowed RawFd from `master_writer`. The input thread later moves `master_writer` and drops it when stdin closes mid-session, so any later TIOCSWINSZ from the forwarder would ioctl on a closed (and possibly reused) descriptor. Hand the forwarder its own try_clone of the master File and leak it inside, so the resize fd has the same lifetime as the signal handler that targets it. Addresses Copilot review feedback on PR #322. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

…aths Verified empirically on macOS 15 (Apple Silicon) with sandbox-exec: * posix_openpt + grantpt + unlockpt + ptsname + slave open all succeed under a profile that allows pseudo-tty and /dev/ptmx but NOT iokit-open. Removed the broad (allow iokit-open) from write_nested_pty_rules. * SecItemAdd / SecItemCopyMatching against a generic-password item succeed without iokit-open as long as the Mach surface (securityd, trustd, ocspd, cfprefsd, xpcd, lsd.*) and the filesystem surface (~/Library/Keychains, /private/var/folders, /private/var/db/mds) are reachable. Removed (allow iokit-open) from write_keychain_rules. * /Library/Keychains and /System/Library/Keychains are already covered by the baseline (subpath "/Library") and (subpath "/System") read-only allows. Removed the redundant subpath rules. This drops every IOKit user-client class from the keychain + nested pty paths, so HID injection stays denied without needing the last-match-wins ordering trick. Removed the now-stale ordering tests and the now-stale comment on build_profile. Docs updated to match. The HID iokit deny that ui.injection: false emits is unchanged. Addresses richiemsft + Copilot review feedback on PR #322. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

…reference * sdk/src/types.ts: add nestedPty and keychainAccess to SeatbeltConfig with JSDoc matching the JSON schema. Without this, TypeScript callers using ContainerConfig.experimental.seatbelt couldn't set the new fields without a cast. * docs/schema.md: extend the experimental.seatbelt example to include every option (profileOverride, guiAccess, launchMethod, nestedPty, keychainAccess) so the reference matches the JSON schema. Addresses Copilot review feedback on PR #322. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

…fix/pty-sigwinch-keychain

caarlos0 and others added 4 commits May 15, 2026 12:29

Revert "feat(mxc-darwin): bump RLIMIT_NOFILE to hard cap"

2270cbe

This reverts commit 6b8c32d.

Copilot AI review requested due to automatic review settings May 15, 2026 15:57

Copilot started reviewing on behalf of caarlos0 May 15, 2026 15:58 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

caarlos0 and others added 3 commits May 15, 2026 13:48

docs: update

0105beb

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

caarlos0 and others added 2 commits May 15, 2026 13:58

Merge remote-tracking branch 'upstream/main' into fix/pty-sigwinch-ke…

1282772

…ychain

caarlos0 marked this pull request as ready for review May 15, 2026 18:32

Merge branch 'main' into fix/pty-sigwinch-keychain

c293207

richiemsft reviewed May 15, 2026

View reviewed changes

caarlos0 and others added 4 commits May 15, 2026 18:57

Merge remote-tracking branch 'origin/fix/pty-sigwinch-keychain' into …

fd99f5d

…fix/pty-sigwinch-keychain

richiemsft approved these changes May 15, 2026

View reviewed changes

bbonaby merged commit 72f72bb into microsoft:main May 16, 2026
17 checks passed

caarlos0 deleted the fix/pty-sigwinch-keychain branch May 18, 2026 12:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: nested ptys, forward sigwinch, keychain access#322

feat: nested ptys, forward sigwinch, keychain access#322
bbonaby merged 14 commits into
microsoft:mainfrom
caarlos0:fix/pty-sigwinch-keychain

caarlos0 commented May 15, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

caarlos0 commented May 15, 2026

Uh oh!

richiemsft May 15, 2026

Uh oh!

caarlos0 May 15, 2026

Uh oh!

richiemsft May 15, 2026

Uh oh!

caarlos0 May 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

caarlos0 commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📖 Description

🔗 References

🔍 Validation

✅ Checklist

📋 Issue Type

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

caarlos0 commented May 15, 2026

Uh oh!

richiemsft May 15, 2026

Choose a reason for hiding this comment

Uh oh!

caarlos0 May 15, 2026

Choose a reason for hiding this comment

Uh oh!

richiemsft May 15, 2026

Choose a reason for hiding this comment

Uh oh!

caarlos0 May 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

caarlos0 commented May 15, 2026 •

edited

Loading